Fix warning with torch.meshgrid #8090

pallgeuer · 2022-05-30T09:33:19Z

Motivation

torch.meshgrid() has started raising a warning when not called with an explicit indexing parameter:
https://pytorch.org/docs/stable/generated/torch.meshgrid.html

python demo/image_demo.py demo/demo.jpg configs/yolox/yolox_s_8x8_300e_coco.py https://download.openmmlab.com/mmdetection/v2.0/yolox/yolox_s_8x8_300e_coco/yolox_s_8x8_300e_coco_20211121_095711-4592a793.pth

gives

/home/allgeuer/anaconda3/envs/mmdev/lib/python3.9/site-packages/torch/functional.py:445: UserWarning: torch.meshgrid: in an upcoming release, it will be required to pass the indexing argument. (Triggered internally at  ../aten/src/ATen/native/TensorShape.cpp:2157.)
  return _VF.meshgrid(tensors, **kwargs)  # type: ignore[attr-defined]

Modification

Provide indexing='ij' to all calls to torch.meshgrid that don't already specify indexing.

ZwwWayne · 2022-05-30T09:40:49Z

Hi @pallgeuer ,
Thanks for your kind contribution. Since PyTorch only support indexing argument of meshgrid 1.10. Simply update the interface will cause mmdet cannot run on PyTorch<1.10. However, we need to support PyTorch >=1.5. Could you implement a meshgrid function to wrap torch.meshgrid? In this function, you can handle both cases so that mmdet can still run on PyTorch < 1.10.
For example, implement and use the function below:

def meshgrid(x, y, indexing=None):
    # if pytorch version < 1.10:
    # use torch.meshgrid
    # else: use torch.meshgrid(x, y, indexing=xx) to avoid warning.

pallgeuer · 2022-05-30T10:22:48Z

Good point. This affects many packages (mmdetection, mmpose, mmclassification, etc). How to best deal with that?
Create a wrapper for torch.meshgrid in mmcv?

ZwwWayne · 2022-05-31T01:36:11Z

Good point. This affects many packages (mmdetection, mmpose, mmclassification, etc). How to best deal with that?

Create a wrapper for torch.meshgrid in mmcv?

Since we do not want to increase the dependency on mmcv so fast, we can add a wrapper in mmcv first, then we can also copy the wrapper in mmdet and use that in mmcv if the mmcv version meets our demand, otherwise we can use that in mmdet.

pallgeuer · 2022-05-31T08:15:15Z

Where would be the appropriate file/location in mmcv and mmdet to add such wrappers?

pallgeuer · 2022-06-07T09:41:44Z

I have updated the PR with the same solution as was merged for mmpose:
open-mmlab/mmpose#1402

ZwwWayne · 2022-06-07T11:34:02Z

mmdet/utils/misc.py

@@ -74,3 +76,14 @@ def update(cfg, src_str, dst_str):

    update(cfg.data, cfg.data_root, dst_root)
    cfg.data_root = dst_root
+
+
+_torch_version_meshgrid_indexing = version.parse(


Following the version comparison style in MMCV https://github.com/open-mmlab/mmcv/blob/235c0253ab8806a2a2ee6954b4258a95358497ac/mmcv/parallel/distributed.py#L40, it might be better to write the code as below:

from mmcv.utils import TORCH_VERSION, digit_version if ('parrots' not in TORCH_VERSION and digit_version(TORCH_VERSION) >= digit_version('1.10')

I can't seem to find much information on the parrots version of PyTorch - what should it do in that case? Does the torch.meshgrid method for parrots PyTorch support the indexing parameter or not and/or does it raise a warning?

I had a look and couldn't immediately identify what exactly digit_version does differently. In what case would the result of a version comparison like I coded it and your proposal differ?

digit_version('1.10') is not quite sufficient by the way, it does really need to be digit_version('1.10.0a0'). My own PyTorch installation is exactly inbetween those two and it makes a difference.

I can't seem to find much information on the parrots version of PyTorch - what should it do in that case? Does the torch.meshgrid method for parrots PyTorch support the indexing parameter or not and/or does it raise a warning?

I had a look and couldn't immediately identify what exactly digit_version does differently. In what case would the result of a version comparison like I coded it and your proposal differ?

digit_version('1.10') is not quite sufficient by the way, it does really need to be digit_version('1.10.0a0'). My own PyTorch installation is exactly inbetween those two and it makes a difference.

It is an internal version that we support. You are right that digit_version does not do anything different, it is just a convention. Then how about using digit_version('1.10.0a0')?

I have updated the PR

codecov · 2022-06-07T11:56:32Z

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (240d7a3) 64.69% compared to head (ea5c9c2) 64.18%.
Report is 153 commits behind head on dev.

Files	Patch %	Lines
mmdet/models/dense_heads/cascade_rpn_head.py	33.33%	2 Missing ⚠️
mmdet/models/utils/transformer.py	33.33%	2 Missing ⚠️
mmdet/models/dense_heads/anchor_free_head.py	50.00%	1 Missing ⚠️
mmdet/models/dense_heads/vfnet_head.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #8090      +/-   ##
==========================================
- Coverage   64.69%   64.18%   -0.52%     
==========================================
  Files         351      361      +10     
  Lines       28463    29537    +1074     
  Branches     4807     5020     +213     
==========================================
+ Hits        18414    18958     +544     
- Misses       9057     9575     +518     
- Partials      992     1004      +12

Flag	Coverage Δ
unittests	`64.15% <66.66%> (-0.54%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ZwwWayne · 2022-06-09T02:05:33Z

Hi. @pallgeuer ,
Thanks for your kind contribution.
We have discussed the naming issue and wish the code to be changed as below:

_torch_version_meshgrid_indexing = (
    'parrots' not in TORCH_VERSION
    and digit_version(TORCH_VERSION) >= digit_version('1.10.0a0'))


def torch_meshgrid(*tensors):
    """A wrapper of torch.meshgrid to compat different PyTorch versions.
    
    Since PyTorch 1.10.0a0, torch.meshgrid supports the arguments ``indexing``.
    So we implement a wrapper here to avoid warning when using high-version PyTorch and avoid compatibility issues when using previous versions of PyTorch.

    Args:
        xxx: (add description here for each argument)

    Return:
        
    """
    if _torch_version_meshgrid_indexing:
        return torch.meshgrid(*tensors, indexing='ij')
    else:
        return torch.meshgrid(*tensors)  # Uses indexing='ij' by default

The docstring should be completed following the google docstring guide.
To reduce the repeat work on your side, we suggest you directly update the PR in MMCV and we will review and merge it quickly.
Then we can help to update those interfaces in the remaining repositories like mmdet/mmpose/mmocr.

Thank you!

pallgeuer · 2022-06-09T10:13:00Z

I have updated the mmcv PR.
I guess this means that the mmpose/etc packages will all have to depend on the latest version of mmcv soon then.

ZwwWayne · 2022-06-11T06:10:26Z

Hi @pallgeuer ,
Thanks for your kind and patient contribution. I can imagine that it is not easy to process multiple similar PR across repos.
We will take over the following progress in MMCV and downstream repos. Furthermore, we will update our contribution documentation to make it more friendly to the community.

ZwwWayne · 2023-01-29T11:32:50Z

MMCV already supports torch_meshgrid wrapper since 1.6.0 https://github.com/open-mmlab/mmcv/releases/tag/v1.6.0. We can upgrade the minimum requirement of MMCV since this PR.

Fix warning with torch.meshgrid

8236173

mm-assistant bot assigned Czm369 May 30, 2022

This was referenced May 30, 2022

Fix warning with torch.meshgrid open-mmlab/mmtracking#577

Open

Fix warning with torch.meshgrid open-mmlab/mmpose#1402

Merged

[Fix] Fix warning with torch.meshgrid. open-mmlab/mmpretrain#860

Merged

ZwwWayne added this to the 2.25.1 milestone Jun 1, 2022

Add torch_meshgrid_ij utility

31de374

pallgeuer mentioned this pull request Jun 7, 2022

Add torch_meshgrid_ij wrapper due to PyTorch change open-mmlab/mmcv#2044

Merged

7 tasks

ZwwWayne reviewed Jun 7, 2022

View reviewed changes

Use digit_version instead of version.parse

ea5c9c2

ZwwWayne removed this from the 2.25.1 milestone Aug 15, 2022

ZwwWayne changed the base branch from master to dev January 29, 2023 11:32

ZwwWayne assigned zwhus and unassigned Czm369 Jan 29, 2023

ZwwWayne added this to the 2.29.0 milestone Jan 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix warning with torch.meshgrid #8090

Fix warning with torch.meshgrid #8090

pallgeuer commented May 30, 2022

ZwwWayne commented May 30, 2022

pallgeuer commented May 30, 2022

ZwwWayne commented May 31, 2022

pallgeuer commented May 31, 2022

pallgeuer commented Jun 7, 2022

ZwwWayne Jun 7, 2022

pallgeuer Jun 7, 2022

ZwwWayne Jun 8, 2022

pallgeuer Jun 8, 2022

codecov bot commented Jun 7, 2022 •

edited

Loading

ZwwWayne commented Jun 9, 2022 •

edited

Loading

pallgeuer commented Jun 9, 2022

ZwwWayne commented Jun 11, 2022

ZwwWayne commented Jan 29, 2023

Fix warning with torch.meshgrid #8090

Are you sure you want to change the base?

Fix warning with torch.meshgrid #8090

Conversation

pallgeuer commented May 30, 2022

Motivation

Modification

ZwwWayne commented May 30, 2022

pallgeuer commented May 30, 2022

ZwwWayne commented May 31, 2022

pallgeuer commented May 31, 2022

pallgeuer commented Jun 7, 2022

ZwwWayne Jun 7, 2022

Choose a reason for hiding this comment

pallgeuer Jun 7, 2022

Choose a reason for hiding this comment

ZwwWayne Jun 8, 2022

Choose a reason for hiding this comment

pallgeuer Jun 8, 2022

Choose a reason for hiding this comment

codecov bot commented Jun 7, 2022 • edited Loading

Codecov Report

ZwwWayne commented Jun 9, 2022 • edited Loading

pallgeuer commented Jun 9, 2022

ZwwWayne commented Jun 11, 2022

ZwwWayne commented Jan 29, 2023

codecov bot commented Jun 7, 2022 •

edited

Loading

ZwwWayne commented Jun 9, 2022 •

edited

Loading